Skip to content

Add cron trigger capability#7

Merged
justinkaseman merged 8 commits into
mainfrom
jk/cron-trigger
Aug 29, 2024
Merged

Add cron trigger capability#7
justinkaseman merged 8 commits into
mainfrom
jk/cron-trigger

Conversation

@justinkaseman

@justinkaseman justinkaseman commented Aug 26, 2024

Copy link
Copy Markdown
Contributor

Note

This PR moves over the chainlink-common PR: smartcontractkit/chainlink-common#715

It depends on Trigger API changes. Draft PR until they are merged.


const cronTriggerID = "cron-trigger@1.0.0"

// TODO pending capabilities configuration implementation - this should be configurable with a sensible default

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has already been implemented, hasn't it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet ☹️

Comment thread capabilities/cron/trigger/cron_trigger.go Outdated
Comment thread capabilities/cron/trigger/cron_trigger.go Outdated
Comment thread capabilities/cron/main.go Outdated
Comment thread capabilities/cron/trigger/cron_trigger.go Outdated
Comment thread capabilities/cron/trigger/monitor.go Outdated
// Time that cron trigger's task execution occurred (RFC3339Nano formatted)
ActualExecutionTime string
// Time that cron trigger's task execution had been scheduled to occur (RFC3339Nano formatted)
ScheduledExecutionTime string

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justinkaseman One idea to improve this is to build support for time objects in the values library. I think this makes a lot of sense and would be useful for other use cases; atm we work around this via this trick, but it's a bit hacky.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. I'll make a ticket for this.

@justinkaseman justinkaseman marked this pull request as ready for review August 28, 2024 17:34
@justinkaseman justinkaseman added this pull request to the merge queue Aug 29, 2024
Merged via the queue into main with commit 71cba15 Aug 29, 2024
@justinkaseman justinkaseman deleted the jk/cron-trigger branch August 29, 2024 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants